@PX4/testflights this needs testing, but we should sort out your radio issues first.

Scenarios to test and review (from logs).

1. SiK radio with and without mavlink RADIO_STATUS working
   • Mavlink v1/v2 autonegotiate with Sik radio review #10358
2. SiK radios with and without flow control physically connected

Enable logging from boot so that the log contains the initial parameter and mission sync. Then do a short flight so we can see how the mavlink module responds (gracefully degrading or not).

Looks good to me, we just need to make sure we're not missing side effects. @bkueng Could you please also have a look?

We'll need to check the worst case mavlink packet size (largest + signing) that we'd like to support.

A simple & clean solution would be to change MAVLINK_START_UART_SEND to return a bool, and only call the _mavlink_send_uarts if it returns true. (https://github.com/mavlink/c_library_v2/blob/master/mavlink_helpers.h#L349)

Actually I was thinking of doing that as well to catch the sends not coming from a stream update call. We'd need to find a way to introduce it as a non-breaking change to the upstream project (https://github.com/ArduPilot/pymavlink/blob/master/generator/C/include_v2.0/mavlink_helpers.h#L348-L355). On the PX4 side I'd still like to skip the work and save the data when possible.

There are race conditions where it will fail. However I see no correctnes errors.

Could you elaborate? I'm aware of the general disconnect between mavlink deciding it's time to send and the actual uorb data availability. Again, I'd strongly prefer to not throw away good data.

signature header length is not included in get_size()

This needs to be fixed regardless. Are you aware of any actual mavlink signing usage?

couple flights on a pixracer (v4)
https://logs.px4.io/plot_app?log=bd9b756e-495e-4cfb-8936-4df94e3bb0dd
https://logs.px4.io/plot_app?log=290eed2d-7124-415c-a9cc-a79bdbbd024b

unable to establish proper connection; probably a hardware issue with SiK radio

As discussed offline, @santiago3dr's telemetry radios seem to have a severe problem unrelated to this PR. We'll reconvene for testing once there's a stable baseline.

got the new telemtry radios which are getting good range.
did a range test on master 39221f
https://logs.px4.io/plot_app?log=809addb6-df5a-4d30-bd54-c619292c73e9
https://logs.px4.io/plot_app?log=9225eeca-4886-4ff5-adf9-393c768a151a

ran the same range test on this pr and rtl kept getting rejected; disconnected telemtry radio and still not able to engage rtl
https://logs.px4.io/plot_app?log=43e6ccdf-8ad2-46fb-951d-70ffe2d03ab9

@dagar thoughts?

@santiago3dr RTL failed because you didn't have a valid home position.

I've rebased the PR on master, could you try again?

@santiago3dr Could you re-test? Thanks!

mixed results
had difficulty maintaining connection and loading parameters
https://logs.px4.io/plot_app?log=6273d7fb-eb7c-4d45-9c5b-128d057b6d0b

great connection; better connection than with master
https://logs.px4.io/plot_app?log=6daf5df5-88b2-4ecb-b7d4-acbb4402ad91

same test and environment on master cfac2cc for comparison:
https://logs.px4.io/plot_app?log=0cd4d221-887e-4109-86e1-30c8c9ed6d69

@santiago3dr your radio RSSI looks much better, but in all of your logs mavlink is still reporting flow control disabled.
How is the radio wired? Check RTS/CTS.

The hlybro radio's we got only have 4 wire connectors so no go on CTS and RTS
http://www.holybro.com/product/57

Well that's unfortunate. We have some other ideas that will help, but nothing immediate. This is all kind of secondary to this PR though.

@dogmaphobic FYI

@santiago3dr could you try newer QGC daily? With only telemetry connected take a look at "Loss rate" under Settings -> Mavlink.

stable 1~2% loss rate

@dagar What's your conclusion here?

@dagar What's your conclusion here?

That this change is probably fine, but only addresses a subset of the many problems. I'll open another issue.

Update: new issue created #10618

Likely worth dropping this PR and starting again, but tagging @MaEtUgR @jkflying for historical reference.

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

Closing as stale.